Rename module to github.com/bytevet/godzilla so go install works - #45

The frontend parsed twice: esbuild's api.Transform normalised TS/JSX/ESM to
text, goja reparsed that text, and a sourcemap carried positions back. esbuild
had already built an AST and discarded it -- its parser is unreachable under
internal/, so github.com/bytevet/esbuild-jsast re-exports it and the lowering
now consumes it directly. goja, evanw/esbuild and go-sourcemap all leave go.mod.

The round trip cost precision, not just time. Printing was ~79% of the
transform, and goja's ragged grammar forced downlevels that rewrote object
spread into a helper taint did not survive. Top-level await could not work at
all: it is expressible only in ESM output, which a CommonJS-consuming lowering
cannot read. It now parses, and its known-gap fixture moves into dialects/.

Positions are correct by construction rather than reconstructed. A node's Loc
is a byte offset into the buffer as written, so remapPositions and the sourcemap
dependency are gone; lineIndex is the one place an offset becomes a position,
using ECMAScript's line terminators (LF, lone CR, CRLF as one, U+2028/9) and
1-based byte columns -- goja's exact convention, so nothing moves. Offset 0 is
a VALID position, so "no node" is decided by a nil Data, never by a zero offset.

The two-axis ladder collapses to one. Loader selection remains driven by parse
failure rather than prediction, since one extension covers four dialects, and
flowstrip stays its last rung. ESM is now native: import clauses feed the same
alias table require() does, so a module-anchored sink rule matches an imported
binding. The comma-callee case survives retargeted -- our own pipeline no longer
emits `(0, mod.fn)(x)`, but every bundler does.

Verified against a position oracle captured from the pre-change build: all 58
test/js findings, byte-identical source and sink line/column. The one delta is
esm_module's enclosing function, $anon1 -> $anon0, because esbuild's injected
__export helper arrow no longer consumes index 0 -- the new name is the correct
one. That oracle is now a permanent gate (TestJSFindingPositions): expected.yaml
asserts counts and only nine JS samples assert a line, so a systematic column
shift previously passed the corpus in silence.

Also pinned: parser options cannot request a transform (Options is two
booleans), dead branches and consts survive parsing, lineIndex handles CRLF and
U+2028/9, and Skipped() over test/js stays exactly 1. gojacaps_test.go is
deleted -- it asserted a goja/esbuild disagreement that no longer exists -- with
every construct it covered moved into testdata/dialects/, where
TestDialectsAllConvert makes the stronger claim. js.unsupported across test/js
and testdata/dialects is zero. 90s of fuzzing, 373k execs, no panic.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rao1GBomZ53fL2ydytD96r

Cleanup pass over the esbuild-AST rewrite. No behaviour change: all 58 test/js
findings keep byte-identical source and sink positions.

Four helpers collapsed into one. identName, calleeIdentName and
assignTargetName had the same body, differing only in where the *jsast.File
came from, and mustParseOK duplicated mustParse. Identifiers reach the tree as
symbol Refs, so the next time esbuild spells one a new way it should be one
edit, not four.

sfc.go's lineColOf was a second offset-to-line/column walk that knew only LF,
so a CRLF or U+2028 component numbered its template directives on different
rules than its script body -- exactly the silent shift the position golden was
written to catch, in the one place it does not look. It now shares lineIndex,
which also drops findBlock's openTag return, dead since tagHasTS went.

isDialectExt no longer decided anything: nothing gates on "the extension
narrows the dialect" since parseLadder owns that, and IsJSFamily's doc still
claimed a derivation the code had stopped doing. The extension set is now
written once.

Two invariants were resting on prose and now have tests:

  - Ladder rung ORDER. `f(a < b, c > (d))` parses cleanly under BOTH the JS and
    TS rungs and they disagree: JS reads two relational arguments, TS reads one
    argument with a type-argument list. Try TS first and an argument, with
    whatever taint flowed through it, leaves the IR with nothing failing to
    parse. Verified the new test fails when the rungs are swapped.
  - ModeBundle. jsast.Options names four hazards; the alarm covered three. The
    missing one rewrites require/import into linker shape, which is precisely
    what both alias tables pattern-match, so every module-anchored sink would
    stop matching while files still parsed.

Also fixed a trap of my own making: the position gate regenerated its own
golden under GODZILLA_REGEN=1 and returned early, so the documented regen
command reported the gate green on exactly the run meant to show the damage.
Regeneration moved to regen_test.go with the manifests; the gate only asserts.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rao1GBomZ53fL2ydytD96r

Two silent false negatives, found while auditing what the esbuild migration
made obsolete. Little was obsolete -- the migration deleted its own dead paths
-- but the audit turned these up, both pre-existing.

The collector walked a loop's BODY while the lowering lowers its HEADER, so a
function literal in a header was never named: the lowering had nothing to
resolve, emitted js.unsupported, and the literal's body went unanalyzed while
the file still reported as converted.

  for (const row of req.query.rows.filter(v => v.ok)) { exec(row.cmd); }
  -> functions: 2 (the arrow is missing), js.unsupported: 1

Parameter defaults had the same hole and were quieter still -- nothing lowers
them, so a literal there produced no intrinsic to notice, just an unanalyzed
function. Both are now collected.

TestNoUnsupportedInstructions is why this survived: it named six files, and a
hand-maintained list only ever covers yesterday's shapes. It now walks test/js
and testdata/dialects whole. Verified it fails on the reverted collector.

Separately, resolveRelativeModule stripped only a case-sensitive set of
extensions that omitted .vue/.svelte, while walkignore.ModuleName strips any
extension in any case. So `import C from './Comp.vue'` resolved to module
"Comp.vue" against a file lowered as "Comp" and the cross-module edge vanished
-- coverage=ok, no skipped file, no error, just a finding that stopped being
reported. IsJSFamily now decides, which is the same predicate that named the
target module. Blanket-stripping would be wrong: `./config.prod` is an
extensionless import of config.prod.js.

Documentation was carried across the migration unexamined -- the whole "Known
limitations" block is byte-identical to its pre-migration text. Corrected the
two entries that were wrong (destructuring understated object patterns, which
bind per key, while denying that for-of heads bind nothing; function-literal
discovery is no longer a limitation and is now stated as the collector-coverage
invariant it actually is), and swept ~13 sites of prose that still described
goja, a CommonJS lowering, a sourcemap, or esbuild's Loader enum. Dropped
CLAUDE.md's copy of the rung order, which the code states and a test pins.

flowstrip.go keeps its code. Measured at ~73% load-bearing: most Flow
annotations ARE valid TypeScript and never reach it, but ?T, casts, <T: B> and
opaque type do, and the guards that look redundant are what make the rest safe.
Its prose was wrong in the other direction and is corrected, including a
recorded gap: class-body variance is stepped over, never blanked.

Verified: 58 pre-existing findings keep byte-identical positions; the only diff
is the two findings these fixes recover. Fuzz 198k execs, no panic. Corpus has
zero non-Java failing subtests (Java is JDK 21 vs 24, environmental).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rao1GBomZ53fL2ydytD96r

Comment bloat, not code bloat: the previous commit added 69 comment lines to
19 code lines. This removes 27 comment lines from Go and 9 more from fixtures.

"The collector must walk what the lowering lowers" was written in six places.
It now has one home in converter.go's package doc; funcRefValue and the loop
arms point at it. The addFunction comment stays because it carries a DIFFERENT
fact -- nothing lowers a parameter default -- and the fixture headers hand
their rationale to expected.yaml, where someone deleting a sample looks.

That duplication had already started to disagree with itself. The package doc
claimed TestNoUnsupportedInstructions was the alarm for an uncollected literal,
which is false for a parameter default: nothing lowers one, so no intrinsic is
emitted and the test sees nothing. Half the previous commit's fix was therefore
shipped untested. TestCollectsParamDefaultLiteral now covers it, and was
verified to fail with the collector loop removed while TestNoUnsupportedInstructions
stayed green -- which is the blind spot, demonstrated.

Also:

  - collectStmts(stmtList(x)) is collectStmt(x): collectStmt already flattens
    SBlock and no-ops on a nil Data. Twelve call sites, one wrapper gone.
    stmtList stays for lower.go, which genuinely wants a slice.
  - TestNoUnsupportedInstructions uses irwalk instead of hand-rolling a
    four-deep Modules/Functions/Blocks/Instrs nest, in a file that already
    imports it.

Net Go LoC is +8, not negative: the new test costs ~25 lines and the trimming
saved ~17. Coverage was the right trade, but it is a trade, not a win.

SForIn/SForOf stay duplicated -- Go cannot bind a typed value across a
multi-type case, and a helper costs more lines than it saves; the file already
spells SWhile/SDoWhile the same way.

Positions: three findings shift, in exactly the three fixtures whose headers
shrank, by exactly the lines removed. Nothing else moved.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rao1GBomZ53fL2ydytD96r

CI's Lint & format job has been red since 692ba9a on one staticcheck issue:

  parseopts_test.go:143: ST1018: string literal contains the Unicode format
  character U+2060, consider using the '⁠' escape sequence instead

The test case is right -- U+2060 (e2 81 a0) shares U+2028's lead byte without
being a line break, which is exactly what lineIndex must not confuse -- but
writing it as a raw glyph puts an invisible character in the source. It is now
the escape. Behaviour is unchanged; the case still asserts {2, 1} at offset 7.
A repo-wide scan finds no other category-Cf character in a Go source.

The reason it stayed red is the second half of this commit: `make gate` was
`fmt vet build test`, so the gate this repo documents could not catch a lint
regression, and `go vet` does not include staticcheck. gate now runs lint too.

Reproducing CI locally has a sharp edge worth recording in the Makefile:
golangci-lint refuses to run at all when built with an older Go than the module
targets, and reports it as a config-load error rather than a lint result, so an
outdated binary looks like a broken repo. The installed 2.5.0 hit exactly that,
and `go install ...@v2.12.2` still does unless GOTOOLCHAIN forces 1.26.5.

Verified with CI's own version (2.12.2, built with go1.26.5): 0 issues.

The other three CI jobs were already green, including Build & test
(Go/Python/JS/Java/Rust) -- so the Java corpus failures seen locally are the
JDK 21-vs-24 environment difference and nothing else.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rao1GBomZ53fL2ydytD96r

lineIndex inherited ECMAScript's line-terminator set from goja, which is right
for a GRAMMAR -- U+2028/U+2029 do end a statement -- and wrong for a reported
position. Since ES2019 both are legal inside a string literal, and every
consumer of the number lineIndex produces splits on "\n": srclines, which feeds
the HTML report snippet, the LLM reviewer's context, and godzilla:ignore
matching in internal/triage.

So one U+2028 in a string shifted every later finding in that file off its own
text. Demonstrated before the change:

  const BANNER = "line-sep:<U+2028>here";
  ...
  exec(req.query.cmd);        // \n-split line 4; reported as line 5

The report would render line 5 -- `}` -- as the vulnerable line, the reviewer
would adjudicate against `}`, and an ignore directive on the real line would not
match. The offsets were never wrong; deriving a line from them was.

CRLF still counts once and lone CR still breaks: those agree with a \n split
(and with editors), and double-counting CRLF would shift every line of every
file written on Windows. That is the case in this function that genuinely earns
its keep.

No corpus position moves -- no fixture contains either character -- so the
position golden is untouched.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rao1GBomZ53fL2ydytD96r

Seven test functions, none of which could fail in a way something else did not
already catch. -69 lines in converters/javascript, no coverage lost.

  TestLogXSSSampleInstructions      no assertions at all -- pure t.Logf, so it
                                    could never fail
  TestConvertCommandInjectionSample \
  TestConvertSQLInjectionSample      | same four samples, same requireFinding
  TestConvertSSRFSample              | call, that TestNewRulePacksDoNotCrossFire
  TestConvertPathTraversalSample    /  already makes -- plus a cross-fire check
  TestConvertDirectory              "at least 2 modules from test/js", implied
                                    by the same tree converting with 1 skip
  TestNoUnsupportedInstructions     folded, see below

TestConvertXSSSample stays: unlike its four siblings it also pins the module
count, the language tag and a canonical name.

The fallback-intrinsic walk moves into the two tests that were ALREADY
converting those trees, as requireNoFallbackIntrinsic. "Skipped is 1" and "no
instruction lowered to js.unsupported" are one invariant -- this tree converted
completely -- and asserting them over one conversion instead of three drops two
full walks of test/js from the package's runtime.

Verified the relocation kept its teeth: removing the SForOf header collection
again fails TestConvertCorpusTreeSkipsOnlyBroken, naming the same function.

test/corpus was reviewed and left alone. Its four suites look overlapping and
are not: signal/noise scores the labeled corpus, the differential and
multi-language suites cover shapes no single-language sample can, and
TestSampleModulesBuild compile-checks the isolated Go modules that `go test
./...` skips entirely.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rao1GBomZ53fL2ydytD96r